Skip to content

Conversation

@danceb
Copy link
Contributor

@danceb danceb commented Apr 1, 2025

This PR contains two adjustments.

  1. Environmental variables are always strings, so they has to be converted to integers for using within create_engine().
  2. The initial PR will activate db pooling for everyone. I think it is better to let the user choose, if it should be used or not by setting another ENV.

Feel free to release a new package version of qwc-services core or just rebuild 1.4.5 (?)

@manisandro manisandro merged commit 65c4a2e into qwc-services:master Apr 3, 2025
@manisandro
Copy link
Member

Thanks

@tpo
Copy link
Member

tpo commented Apr 3, 2025

A question @danceb : feel free to comment or not ... how you wish...

You write:

The initial PR will activate db pooling for everyone. I think it is better to let the user choose, if it should be used or not by setting another ENV.

If using pooling were an improvement for everyone, then why wouldn't we want to have it as a default?

Or asked in another way:

  • does pooling improve things for everybody?
  • are there downsides to pooling?

@danceb
Copy link
Contributor Author

danceb commented Apr 3, 2025

In my opinion pooling would be an improvement for everyone, otherwise I would not recommended this as PR ;-)

Otherwise setting the optimal values of the parameters for POOL_SIZE, MAX_OVERFLOW etc. depends on the individual application and could cause into troubles, as you mentioned by yourself in #14 (comment).

So I wouldn't go with it, to set the default values initially for anybody.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants